-
Couldn't load subscription status.
- Fork 3.6k
Playground Editor V2: Multi-file, ES modules, NPM Support + Build output hashing #17216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…styling, inline definitions and navigation to symbols
…nnable compiled by monaco ts worker
|
You have changed file(s) that made possible changes to the sandbox. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/17216/merge/ |
|
Devhost visualization test reporter: |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
I did a quick test of Playground, but it looks like Inspector v2 is not working at all. It looks like there are still merge issues on these lines:
|
|
Hah sorry had been testing the negative case with the old version, thanks for the detailed lineup, that did it. Should be back to 1:1 |
|
Reviewer - this PR has made changes to one or more package.json files. |
|
You have changed file(s) that made possible changes to the sandbox. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/17216/merge/ |
|
You have made possible changes to the playground. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/17216/merge/ The snapshot playground with the CDN snapshot (only when available): Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly. |
|
Devhost visualization test reporter: |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
|
@knervous I'm about to merge. Good on your side buddy? |
All good here, I'll be offline until early tomorrow morning Berlin time but will be available at any point after that to troubleshoot any issues. Thanks for helping get this through to the finish line (again) 🥳 |
|
Dude! Thank you my friend! This is a MAJOR update we are all proud of! |
Very happy to have made this contribution :) will work on a Docs PR soon! |
|
You rock |
The Playground [Editor v2](#17216) updated Monaco which regressed the [workaround ](#16741 tab handling, where when Inspector v2 is open, pressing the tab key while focus is in the code editor moves focus instead of inserting a tab character. To work around this, we added the contentEditable attribute to the actual Monaco html element that has keyboard focus when editing text, but that element has changed in the newer Monaco, so the workaround needs to be updated. Also, the Editor v2 PR removed the comment explaining the workaround, so I just re-added it from the original change.
…put hashing (BabylonJS#17216) Follow-up to the original PR BabylonJS#17175 The different issues pointed out by different people on the [BJS forum post](https://forum.babylonjs.com/t/babylonjs-playground-multiple-files-with-tabs-esm-npm-imports/60446/44) point to a caching issue - some scripts were loaded fresh while others were stale and cached by the browser. This led to undefined behavior all around and the page would be effectively "broken" depending on which assets were cached or not, i.e. *anything* could go wrong and wouldn't necessarily present as a predictable bug. Previously there were duplicate names from the dist output and monaco worker output which is cached locally in a browser: <img width="1111" height="792" alt="image" src="https://github.com/user-attachments/assets/f0051fe9-8619-4fd7-9549-9ecfc3ba54fc" /> index.html does not have a Cache-Control response header and respects the client's request of `max-age=0` - this is good. index.html should always fetch the latest. It also appears there is a CDN purge script running on the Playground deployment script, although I don't have access to the CI yaml. https://dev.azure.com/babylonjs/ContinousIntegration/_build/results?buildId=44262&view=logs&j=275f1d19-1bd8-5591-b06b-07d489ea915a&t=4d2ea636-6e6e-5831-8b5a-8ce5e9ce8bbc If that's the case for the last operation in that script, good. CDN purge handles the server cache. Takeaway from the existing deployment pipeline and CDN architecture, nothing needs to change. If we can guarantee unique output from a build each time, there is no room for cache confusion from the client's browser. This only applies to the Playground and I haven't looked into other deployed apps. Cache-Control of 2 days is fine. Eventually this issue would've sorted itself out in the field but 2 days is a long time to wait with undefined/broken behavior. The additional changes in this PR are consolidated in these two commits: BabylonJS@1e9c765 knervous@bd197e8 These changes ensure hashed output from every build and deploy for relevant bundled js files. The generated entrypoint for `babylon.playground.[hash].js` is injected into the index.html with the WebpackHTMLPlugin. The helper function for fetching scripts does append a query parameter to bust cache, but dependent chunks of scripts do not automatically follow the same convention. Ideally we can get rid of that `?t=[timestamp]` and let caching do its business. For anyone testing changes, please check https://playgroundv2.vercel.app which should be the exact hosted bundle set from the deployment build. For sanity's sake here are three browsers on mobile tested with the current code: Edge, Firefox and Chrome <img width="216" height="480" alt="image" src="https://github.com/user-attachments/assets/7118d10b-ed77-4fe3-8f48-60f146acdc36" /> <img width="216" height="480" alt="image" src="https://github.com/user-attachments/assets/67873f33-313d-49ce-8070-82dcce064f81" /> <img width="216" height="480" alt="image" src="https://github.com/user-attachments/assets/4fe5ff47-98b3-4f7f-abbd-3a248a469387" /> With editor open: <img width="216" height="480" alt="image" src="https://github.com/user-attachments/assets/9191ac9a-23b0-4d42-943c-2ba9d56127a0" /> --------- Co-authored-by: David Catuhe <[email protected]>
…#17337) The Playground [Editor v2](BabylonJS#17216) updated Monaco which regressed the [workaround ](BabylonJS#16741 tab handling, where when Inspector v2 is open, pressing the tab key while focus is in the code editor moves focus instead of inserting a tab character. To work around this, we added the contentEditable attribute to the actual Monaco html element that has keyboard focus when editing text, but that element has changed in the newer Monaco, so the workaround needs to be updated. Also, the Editor v2 PR removed the comment explaining the workaround, so I just re-added it from the original change.

Follow-up to the original PR #17175
The different issues pointed out by different people on the BJS forum post point to a caching issue - some scripts were loaded fresh while others were stale and cached by the browser. This led to undefined behavior all around and the page would be effectively "broken" depending on which assets were cached or not, i.e. anything could go wrong and wouldn't necessarily present as a predictable bug.
Previously there were duplicate names from the dist output and monaco worker output which is cached locally in a browser:

index.html does not have a Cache-Control response header and respects the client's request of
max-age=0- this is good. index.html should always fetch the latest.It also appears there is a CDN purge script running on the Playground deployment script, although I don't have access to the CI yaml. https://dev.azure.com/babylonjs/ContinousIntegration/_build/results?buildId=44262&view=logs&j=275f1d19-1bd8-5591-b06b-07d489ea915a&t=4d2ea636-6e6e-5831-8b5a-8ce5e9ce8bbc
If that's the case for the last operation in that script, good. CDN purge handles the server cache.
Takeaway from the existing deployment pipeline and CDN architecture, nothing needs to change. If we can guarantee unique output from a build each time, there is no room for cache confusion from the client's browser. This only applies to the Playground and I haven't looked into other deployed apps. Cache-Control of 2 days is fine. Eventually this issue would've sorted itself out in the field but 2 days is a long time to wait with undefined/broken behavior.
The additional changes in this PR are consolidated in these two commits:
1e9c765
knervous@bd197e8
These changes ensure hashed output from every build and deploy for relevant bundled js files. The generated entrypoint for
babylon.playground.[hash].jsis injected into the index.html with the WebpackHTMLPlugin. The helper function for fetching scripts does append a query parameter to bust cache, but dependent chunks of scripts do not automatically follow the same convention. Ideally we can get rid of that?t=[timestamp]and let caching do its business.For anyone testing changes, please check https://playgroundv2.vercel.app which should be the exact hosted bundle set from the deployment build.
For sanity's sake here are three browsers on mobile tested with the current code: Edge, Firefox and Chrome
With editor open: